Skip to content

v4 API: Forms and Landing Pages: Pagination and Total Count - #91

Merged
n7studios merged 13 commits into
v4-api-oauthfrom
v4-api-forms-pagination
Apr 5, 2024
Merged

v4 API: Forms and Landing Pages: Pagination and Total Count#91
n7studios merged 13 commits into
v4-api-oauthfrom
v4-api-forms-pagination

Conversation

@n7studios

@n7studios n7studios commented Apr 1, 2024

Copy link
Copy Markdown
Contributor

Summary

Adds total count and pagination support for get_forms and get_landing_pages methods.

Testing

Existing tests pass. Tests for get_resources method removed, as this method no longer exists, as the v4 API doesn't support the subscription_forms endpoint, and each method (get_forms, get_landing_pages, get_tags) makes its own call to get.

Checklist

@n7studios n7studios added this to the 2.0 milestone Apr 1, 2024
@n7studios n7studios self-assigned this Apr 1, 2024
@n7studios
n7studios requested review from a team, jenessawhite, mercedesb and noelherrick and removed request for a team April 1, 2024 16:15
@n7studios

n7studios commented Apr 1, 2024

Copy link
Copy Markdown
Contributor Author

@noelherrick @mercedesb The PHP SDK has always had a distinction between Forms and Landing Pages (get_forms(), get_landing_pages()), with get_landing_pages() querying the forms endpoint and only returning results that are Landing Pages.

With the introduction of pagination support in the v4 API, get_landing_pages() becomes flaky, given that a per_page or cursor argument might not return any landing pages.

I'm unsure what the best route here is:

  1. Drop pagination support on get_landing_pages. This would require querying the forms endpoint with per_page set to the maximum, and hope that an account hasn't had over 1,000 forms and landing pages in its lifetime.
  2. Drop get_landing_pages as a supported method in the PHP SDK. Consumer / developers can filter results via get_forms if they only wanted to fetch landing pages. I like the convenience of having a get_landing_pages method though (we do something similar across all ConvertKit WordPress Plugins).
  3. Investigate whether it's possible to add a filter to the API to return forms, landing pages or both. I'm unsure of the technical workload needed for this, or whether this is even viable. It would be fantastic if this could be an option; as a use case example, the WordPress Plugin would definitely benefit from it when we come to use the v4 API, as we distinguish between forms and landing pages in some settings there.

Would love to have your input!

@n7studios
n7studios marked this pull request as ready for review April 1, 2024 16:22
@mercedesb

Copy link
Copy Markdown

With the introduction of pagination support in the v4 API, get_landing_pages() becomes flaky, given that a per_page or cursor argument might not return any landing pages.

@n7studios Thanks for calling this out. I have an open PR to be able to filter /v4/forms by either ?type=embed or ?type=hosted.

This should allow you to continue supporting get_landing_pages by using ?type=hosted

It should be merged and deployed within the next day or so.

@noelherrick noelherrick left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - after Mercedes gets that PR merged and you start using those new params, we should be good to go.

@n7studios

Copy link
Copy Markdown
Contributor Author

With the introduction of pagination support in the v4 API, get_landing_pages() becomes flaky, given that a per_page or cursor argument might not return any landing pages.

@n7studios Thanks for calling this out. I have an open PR to be able to filter /v4/forms by either ?type=embed or ?type=hosted.

This should allow you to continue supporting get_landing_pages by using ?type=hosted

It should be merged and deployed within the next day or so.

That's awesome - thanks so much!

@mercedesb

Copy link
Copy Markdown

/v4/forms?type=hosted should work in prod now

@n7studios

Copy link
Copy Markdown
Contributor Author

/v4/forms?type=hosted should work in prod now

@mercedesb Thanks so much!

Would it be possible to add a boolean filter for archived? I've since realised pagination tests are still flaky as the default behaviour of the SDK and WordPress Plugins has always been to remove archived Forms and Landing Pages from the results returned, given we don't need to show them to the end user.

I'm happy with the default behaviour of archived forms being included in results - but any option to filter them out in the API request would be fantastic, so that the per_page and cursor pagination can be supported.

Sorry for not spotting this sooner.

@mercedesb

Copy link
Copy Markdown

Would it be possible to add a boolean filter for archived?

Whoops, we shouldn't have changed behavior. I'll add a status filter. We want to stay away from booleans :) But I'll update it so default behavior is active and then we can filter on the various statuses. The docs will have the full list. My plate is crazy full but I'll try to get to this later today but it might be tomorrow.

@mercedesb

Copy link
Copy Markdown

Ok, you can filter forms by status now. And it'll default to active. Docs have been updated with status options.

@n7studios
n7studios requested a review from noelherrick April 4, 2024 13:18
@n7studios

Copy link
Copy Markdown
Contributor Author

Ok, you can filter forms by status now. And it'll default to active. Docs have been updated with status options.

Awesome, thanks!

@noelherrick have requested a re-review as a few changes since the initial PR, so would be great to have you check this over.

@n7studios
n7studios merged commit 10f8442 into v4-api-oauth Apr 5, 2024
@n7studios
n7studios deleted the v4-api-forms-pagination branch June 26, 2024 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants